fix: Add etag diff suppression to remaining resources by ei-grad · Pull Request #3311 · integrations/terraform-provider-github

ei-grad · 2026-03-30T09:47:21Z

Summary

PR #2840 added DiffSuppressFunc and DiffSuppressOnRefresh to etag fields in 7 resources to prevent spurious diffs on plan/apply. This PR applies the same fix to the remaining 22 resources that were missed:

github_actions_runner_group
github_branch_protection_v3
github_emu_group_mapping
github_enterprise_actions_runner_group
github_issue
github_membership
github_organization_project
github_organization_ruleset
github_organization_webhook
github_project_card
github_project_column
github_release
github_repository_autolink_reference
github_repository_deploy_key
github_repository_ruleset
github_team
github_team_membership
github_team_repository
github_team_sync_group_mapping
github_user_gpg_key
github_user_ssh_key
organization_block

The existing TestEtagSchemaConsistency unit test is extended to cover all 29 resources (7 existing + 22 new).

Fixes #3103
Fixes #3291

… fields PR integrations#2840 added etag diff suppression to 7 resources but missed the remaining 22. This causes spurious diffs on every plan/apply for resources like github_team, github_membership, github_team_repository, github_repository_deploy_key, and others. Apply the same pattern (Optional + DiffSuppressFunc returning true + DiffSuppressOnRefresh) to all remaining resource etag schema fields and extend the unit test to cover all 29 resources. Fixes integrations#3103 Fixes integrations#3291 Co-Authored-By: Claude <[email protected]>

github-actions · 2026-03-30T09:47:33Z

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

deiga · 2026-03-30T16:50:12Z

 			},
 			"etag": {
 				Type:        schema.TypeString,
+				Optional:    true,


issue: setting Optional makes them user editable, we don't want that. Please remove from all resources

Good catch — removed Optional: true from all 29 etag fields (including the 7 from #2840 that also had it) and updated the unit test accordingly.

Though, this is covered in original PR:

Lastly these fields have to be marked as optional=true - [SDK ref](https://github.com/integrations/terraform-provider-github/blob/16872b724254fdddc3441c713b087cb4d7005f83/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/schema.go#L841) Computed-only fields: Computed: true, Optional: false → Can't have DiffSuppressFunc Optional computed fields: Computed: true, Optional: true → Can have DiffSuppressFunc

I added tests that confirm this SDK constraint. TestEtagComputedOnlyRejectsSuppress shows InternalValidate rejects DiffSuppressFunc on Computed-only fields with "There is no config for computed-only field, nothing to compare." TestEtagSchemaConsistency now also runs InternalValidate on all 29 resources to ensure they pass.

Restored Optional: true on all etag fields (including the ones from #2840 that I removed in the previous push).

Removed the standalone SDK validation tests — TestEtagSchemaConsistency now runs InternalValidate on its resources, which covers the same check.

etag is server-computed and should not be user-editable. Remove Optional: true from all etag schema fields (both the 22 new ones and the 7 from PR integrations#2840) and update the unit test accordingly. Co-Authored-By: Claude <[email protected]>

…sFunc The SDK explicitly rejects DiffSuppressFunc on Computed-only fields (InternalValidate returns: "DiffSuppressFunc is for suppressing differences between config and state representation. There is no config for computed-only field, nothing to compare."). Add tests demonstrating this SDK constraint, and verify all 29 resources pass InternalValidate with their current schema. Co-Authored-By: Claude <[email protected]>

…ssFunc The terraform-plugin-sdk v2 InternalValidate rejects DiffSuppressFunc on Computed-only fields with: "There is no config for computed-only field, nothing to compare." Optional: true is required to use DiffSuppressFunc, as demonstrated by the new tests. Co-Authored-By: Claude <[email protected]>

ei-grad · 2026-04-01T10:20:22Z

I don't like that it is copy-pasted everywhere, and that there is no dedicated test which performs InternalValidate for all resources (not just etag'ed-ones). But it feels like these should be handled in separate PRs.

…date in consistency test Co-Authored-By: Claude <[email protected]>

deiga · 2026-04-05T10:24:13Z

+					resourceName: resource,
+				},
+			}
+			if err := p.InternalValidate(); err != nil {


question: this is meant for provider validation and not resource, what is the benefit of adding it?

pdewilde · 2026-04-16T22:41:57Z

It think this should also fix #3085 if you want to add it to the pr description

mtb-xt · 2026-05-04T00:19:21Z

Hey team — I wanted to report a real downstream side effect of this issue and explain how it affects us in practice.

In our CI/CD pipelines, we compare Terraform/OpenTofu planfiles to make sure that the plan approved during PR review is the same plan that gets applied later.

The intent is simple:

The thing we reviewed is the thing we deploy.

To enforce that, we use binary plan comparison through Cloud Posse Atmos:

atmos terraform plan-diff

In our case, this started failing even though Terraform/OpenTofu reported both plans as no-op:

No changes. Your infrastructure matches the configuration.

After converting both planfiles to JSON with:

tofu show -json

…the only meaningful differences were refreshed provider-managed etag values, plus the top-level plan timestamp.

Example:

- "etag": "W/\"9322e152a70cc7591e103fdfe6c5eadb40fbd97f0cde31b018e49ce21f237128\""
+ "etag": "W/\"58d5b901a79908580c54d5f50aa4d4d1e6eb8710448deff8d453c6c17956ca3e\""

Environment where we observed this:

Provider: integrations/github 6.12.1
OpenTofu: 1.11.5
Atmos: 1.213.0
Observed affected resource: github_repository_ruleset

This means a no-op plan is not reproducible. The provider is persisting volatile GitHub API transport/cache metadata into state/plan output, which makes otherwise identical plans compare as different.

That breaks workflows which rely on plan stability, including:

comparing PR-time plans with apply-time plans
verifying that reviewed infrastructure changes are exactly what gets deployed
CI drift/gating workflows based on plan equality

lifecycle.ignore_changes = [etag] does not help here, because this is provider-managed state, not user-controlled configuration.

This PR appears to address the same class of issue we are seeing, including for github_repository_ruleset.

Could a maintainer please either merge this PR, or provide clear direction on what still needs to be changed for it to be accepted? @deiga

Related downstream report with more context:

https://github.com/orgs/cloudposse/discussions/125

Thanks.

github-actions Bot added the Type: Bug Something isn't working as documented label Mar 30, 2026

deiga requested changes Mar 30, 2026

View reviewed changes

ei-grad and others added 3 commits April 1, 2026 09:55

test: Remove standalone SDK validation tests, covered by InternalVali…

c28a943

…date in consistency test Co-Authored-By: Claude <[email protected]>

deiga reviewed Apr 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Add etag diff suppression to remaining resources#3311

fix: Add etag diff suppression to remaining resources#3311
ei-grad wants to merge 5 commits intointegrations:mainfrom
ei-grad:fix-etag-drift-remaining-resources

ei-grad commented Mar 30, 2026

Uh oh!

github-actions Bot commented Mar 30, 2026

Uh oh!

deiga Mar 30, 2026

Uh oh!

ei-grad Apr 1, 2026

Uh oh!

ei-grad Apr 1, 2026

Uh oh!

ei-grad Apr 1, 2026

Uh oh!

ei-grad Apr 1, 2026 •

edited

Loading

Uh oh!

ei-grad commented Apr 1, 2026

Uh oh!

deiga Apr 5, 2026

Uh oh!

pdewilde commented Apr 16, 2026

Uh oh!

mtb-xt commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ei-grad commented Mar 30, 2026

Summary

Uh oh!

github-actions Bot commented Mar 30, 2026

Uh oh!

deiga Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

ei-grad Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ei-grad Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ei-grad Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ei-grad Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ei-grad commented Apr 1, 2026

Uh oh!

deiga Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

pdewilde commented Apr 16, 2026

Uh oh!

mtb-xt commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ei-grad Apr 1, 2026 •

edited

Loading